fix(pool): accept dot-prefixed subdirectories in cwdInWorktree#33
Open
e-jung wants to merge 1 commit into
Open
fix(pool): accept dot-prefixed subdirectories in cwdInWorktree#33e-jung wants to merge 1 commit into
e-jung wants to merge 1 commit into
Conversation
The cwdInWorktree predicate rejected any relative path starting with a dot, so being inside a dot-prefixed subdirectory of a worktree (e.g. .venv/bin, .cache/tmp, .git-hooks) was treated as "not here", causing treehouse status to fail to mark the user as present in the worktree. Replace the rel[0] != '.' check with the same "does not escape via .." predicate already used by internal/process/detect.go: rel must be "." or must not start with "..". This is a status-display-only regression, so the impact is limited to the "you're here" marker. Add the first test for cwdInWorktree as a table-driven test covering the worktree root, regular subdirs, dot-prefixed subdirs (the bug case), and paths outside the worktree.
6932c87 to
b06835e
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Intent
The developer wanted to fix a treehouse bug where cwdInWorktree in internal/pool/pool.go (~line 427) rejected dot-prefixed subdirectories, causing
treehouse statusto fail to recognize the user as "here" when their cwd is inside paths like .venv, .cache, or .git-hooks. They required replacing the buggyrel[0] != '.'check with the correct "does not escape via .." predicate (already present at internal/process/detect.go:75), and adding the first table-driven test for cwdInWorktree in internal/pool/pool_test.go covering root, regular subdir, dot-prefixed subdir, outside-worktree, and sibling-dir cases, verified withgo test ./internal/pool/.... Explicit constraints: branch off upstream/main (never local main), push to the e-jung fork, open a cross-repo PR titled "fix(pool): cwdInWorktree accepts dot-prefixed subdirectories" against kunchenguid/treehouse with AI disclosure = Human-reviewed, do NOT merge, and stay confined to the worktree. The developer framed this as a status-display-only bug with very low risk.What Changed
rel[0] != '.'guard incwdInWorktree(internal/pool/pool.go) with a "does not escape via.." predicate, so dot-prefixed subdirectories like.venv/binor.cache/tmpare correctly recognized as inside the worktree.TestCwdInWorktreecovering worktree root, regular subdir, dot-prefixed subdirs, parent, and sibling paths.Risk Assessment
✅ Low: The fix is a minimal, correct one-liner that mirrors the already-proven predicate in detect.go:75 and is exercised by a well-chosen table-driven test; its only consumer is the StatusHere display flag (pool.go:269), so blast radius is purely cosmetic.
Testing
The fix works end-to-end as an end user would experience it: running
treehouse statusfrom inside.venv/binor.cache/tmpof a pooled worktree now correctly prints "you're here" instead of the buggy "in-use". The new table-driven unit test passes all six cases, the entirego test ./...suite is green, and Windows/macOS cross-builds succeed. A side-by-side run of the buggy base binary versus the fixed binary on the identical scenario directly demonstrates the status-display bug being resolved.Evidence: status transcript - FIXED binary (target commit)
treehouse status from cwd inside dot-prefixed subdirs (fixed binary): --- cwd = worktree/ --- -> 1 you're here --- cwd = worktree/src/foo --- -> 1 you're here --- cwd = worktree/.venv/bin --- -> 1 you're here (was 'in-use' before fix) --- cwd = worktree/.cache/tmp --- -> 1 you're here (was 'in-use' before fix)Evidence: status transcript - BASE (buggy) binary
treehouse status from cwd inside dot-prefixed subdirs (buggy base binary): --- cwd = worktree/ --- -> 1 you're here --- cwd = worktree/src/foo --- -> 1 you're here --- cwd = worktree/.venv/bin --- -> 1 in-use (BUG: should be 'you're here') --- cwd = worktree/.cache/tmp --- -> 1 in-use (BUG: should be 'you're here')Evidence: TestCwdInWorktree unit test result
Evidence: reproducible e2e demo script
Pipeline
Updates from git push no-mistakes
✅ **intent** - passed
✅ No issues found.
🔧 **Rebase** - 1 issue found → auto-fixed ✅
internal/pool/pool_test.go- merge conflict rebasing onto origin/main🔧 Fix applied.
✅ Re-checked - no issues remain.
internal/pool/pool.go:362- The new predicaterel == "." || (rel != ".." && !strings.HasPrefix(rel, ".."+string(filepath.Separator)))is now byte-for-byte identical to the inline check in internal/process/detect.go:75. Since packagepoolalready importsprocess(pool.go:14), the two could share an exported helper (e.g. process.IsPathInsideWorktree) to guarantee the two containment checks never diverge again. The divergence was the root cause of this bug, so consolidating would prevent recurrence. Optional, low-value follow-up given it's a one-liner.✅ **Test** - passed
✅ No issues found.
go test ./internal/pool/... -run TestCwdInWorktree -vgo test ./...GOOS=windows GOARCH=amd64 go build ./...GOOS=darwin GOARCH=arm64 go build ./...e2e: built fixed binary, rantreehouse getto create a pooled worktree, thentreehouse statuswith cwd in worktree root, src/foo, .venv/bin, and .cache/tmp -> all show 'you're here' (status-e2e-FIXED.txt)e2e contrast: built the base-commit (buggy) binary and ran the identical scenario -> .venv/bin and .cache/tmp wrongly show 'in-use' (status-e2e-BASE-buggy.txt), proving the bug and the fix✅ **Document** - passed
✅ No issues found.
✅ **Lint** - passed
✅ No issues found.
✅ **Push** - passed
✅ No issues found.